-
Notifications
You must be signed in to change notification settings - Fork 14.9k
[NFC][HLSL] Remove confusing enum aliases / duplicates #153909
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Remove: * DescriptorType enum - this almost exactly shadowed the ResourceClass enum * ClauseType aliased ResourceClass Although these were introduced to make the HLSL root signature handling code a bit cleaner, they were ultimately causing confusion as they appeared to be unique enums that needed to be converted between. Closes llvm#153890
@llvm/pr-subscribers-clang Author: Damyan Pepper (damyanp) ChangesRemove:
Although these were introduced to make the HLSL Closes #153890 Full diff: https://github.com/llvm/llvm-project/pull/153909.diff 5 Files Affected:
diff --git a/clang/lib/Parse/ParseHLSLRootSignature.cpp b/clang/lib/Parse/ParseHLSLRootSignature.cpp
index 98dc458f7adc5..5490c61f52356 100644
--- a/clang/lib/Parse/ParseHLSLRootSignature.cpp
+++ b/clang/lib/Parse/ParseHLSLRootSignature.cpp
@@ -234,15 +234,15 @@ std::optional<RootDescriptor> RootSignatureParser::parseRootDescriptor() {
default:
llvm_unreachable("Switch for consumed token was not provided");
case TokenKind::kw_CBV:
- Descriptor.Type = DescriptorType::CBuffer;
+ Descriptor.Type = ResourceClass::CBuffer;
ExpectedReg = TokenKind::bReg;
break;
case TokenKind::kw_SRV:
- Descriptor.Type = DescriptorType::SRV;
+ Descriptor.Type = ResourceClass::SRV;
ExpectedReg = TokenKind::tReg;
break;
case TokenKind::kw_UAV:
- Descriptor.Type = DescriptorType::UAV;
+ Descriptor.Type = ResourceClass::UAV;
ExpectedReg = TokenKind::uReg;
break;
}
@@ -360,19 +360,19 @@ RootSignatureParser::parseDescriptorTableClause() {
default:
llvm_unreachable("Switch for consumed token was not provided");
case TokenKind::kw_CBV:
- Clause.Type = ClauseType::CBuffer;
+ Clause.Type = ResourceClass::CBuffer;
ExpectedReg = TokenKind::bReg;
break;
case TokenKind::kw_SRV:
- Clause.Type = ClauseType::SRV;
+ Clause.Type = ResourceClass::SRV;
ExpectedReg = TokenKind::tReg;
break;
case TokenKind::kw_UAV:
- Clause.Type = ClauseType::UAV;
+ Clause.Type = ResourceClass::UAV;
ExpectedReg = TokenKind::uReg;
break;
case TokenKind::kw_Sampler:
- Clause.Type = ClauseType::Sampler;
+ Clause.Type = ResourceClass::Sampler;
ExpectedReg = TokenKind::sReg;
break;
}
diff --git a/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp b/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
index 44f6b0469f38e..44c0978a243bc 100644
--- a/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
+++ b/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
@@ -180,7 +180,7 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseDTClausesTest) {
// First Descriptor Table with 4 elements
RootElement Elem = Elements[0].getElement();
ASSERT_TRUE(std::holds_alternative<DescriptorTableClause>(Elem));
- ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Type, ClauseType::CBuffer);
+ ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Type, ResourceClass::CBuffer);
ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Reg.ViewType,
RegisterType::BReg);
ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Reg.Number, 0u);
@@ -193,7 +193,7 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseDTClausesTest) {
Elem = Elements[1].getElement();
ASSERT_TRUE(std::holds_alternative<DescriptorTableClause>(Elem));
- ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Type, ClauseType::SRV);
+ ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Type, ResourceClass::SRV);
ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Reg.ViewType,
RegisterType::TReg);
ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Reg.Number, 42u);
@@ -205,7 +205,7 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseDTClausesTest) {
Elem = Elements[2].getElement();
ASSERT_TRUE(std::holds_alternative<DescriptorTableClause>(Elem));
- ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Type, ClauseType::Sampler);
+ ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Type, ResourceClass::Sampler);
ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Reg.ViewType,
RegisterType::SReg);
ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Reg.Number, 987u);
@@ -218,7 +218,7 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseDTClausesTest) {
Elem = Elements[3].getElement();
ASSERT_TRUE(std::holds_alternative<DescriptorTableClause>(Elem));
- ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Type, ClauseType::UAV);
+ ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Type, ResourceClass::UAV);
ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Reg.ViewType,
RegisterType::UReg);
ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Reg.Number, 4294967294u);
@@ -445,7 +445,7 @@ TEST_F(ParseHLSLRootSignatureTest, ValidSamplerFlagsTest) {
auto Elements = Parser.getElements();
RootElement Elem = Elements[0].getElement();
ASSERT_TRUE(std::holds_alternative<DescriptorTableClause>(Elem));
- ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Type, ClauseType::Sampler);
+ ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Type, ResourceClass::Sampler);
auto ValidSamplerFlags =
llvm::dxbc::DescriptorRangeFlags::DescriptorsVolatile;
ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Flags, ValidSamplerFlags);
@@ -591,7 +591,7 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseRootDescriptorsTest) {
RootElement Elem = Elements[0].getElement();
ASSERT_TRUE(std::holds_alternative<RootDescriptor>(Elem));
- ASSERT_EQ(std::get<RootDescriptor>(Elem).Type, DescriptorType::CBuffer);
+ ASSERT_EQ(std::get<RootDescriptor>(Elem).Type, ResourceClass::CBuffer);
ASSERT_EQ(std::get<RootDescriptor>(Elem).Reg.ViewType, RegisterType::BReg);
ASSERT_EQ(std::get<RootDescriptor>(Elem).Reg.Number, 0u);
ASSERT_EQ(std::get<RootDescriptor>(Elem).Space, 0u);
@@ -602,7 +602,7 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseRootDescriptorsTest) {
Elem = Elements[1].getElement();
ASSERT_TRUE(std::holds_alternative<RootDescriptor>(Elem));
- ASSERT_EQ(std::get<RootDescriptor>(Elem).Type, DescriptorType::SRV);
+ ASSERT_EQ(std::get<RootDescriptor>(Elem).Type, ResourceClass::SRV);
ASSERT_EQ(std::get<RootDescriptor>(Elem).Reg.ViewType, RegisterType::TReg);
ASSERT_EQ(std::get<RootDescriptor>(Elem).Reg.Number, 42u);
ASSERT_EQ(std::get<RootDescriptor>(Elem).Space, 4u);
@@ -616,7 +616,7 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseRootDescriptorsTest) {
Elem = Elements[2].getElement();
ASSERT_TRUE(std::holds_alternative<RootDescriptor>(Elem));
- ASSERT_EQ(std::get<RootDescriptor>(Elem).Type, DescriptorType::UAV);
+ ASSERT_EQ(std::get<RootDescriptor>(Elem).Type, ResourceClass::UAV);
ASSERT_EQ(std::get<RootDescriptor>(Elem).Reg.ViewType, RegisterType::UReg);
ASSERT_EQ(std::get<RootDescriptor>(Elem).Reg.Number, 34893247u);
ASSERT_EQ(std::get<RootDescriptor>(Elem).Space, 0u);
@@ -628,7 +628,7 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseRootDescriptorsTest) {
RootDescriptorFlags::DataVolatile);
Elem = Elements[3].getElement();
- ASSERT_EQ(std::get<RootDescriptor>(Elem).Type, DescriptorType::CBuffer);
+ ASSERT_EQ(std::get<RootDescriptor>(Elem).Type, ResourceClass::CBuffer);
ASSERT_EQ(std::get<RootDescriptor>(Elem).Reg.ViewType, RegisterType::BReg);
ASSERT_EQ(std::get<RootDescriptor>(Elem).Reg.Number, 0u);
ASSERT_EQ(std::get<RootDescriptor>(Elem).Space, 0u);
@@ -696,17 +696,17 @@ TEST_F(ParseHLSLRootSignatureTest, ValidVersion10Test) {
auto DefRootDescriptorFlag = llvm::dxbc::RootDescriptorFlags::DataVolatile;
RootElement Elem = Elements[0].getElement();
ASSERT_TRUE(std::holds_alternative<RootDescriptor>(Elem));
- ASSERT_EQ(std::get<RootDescriptor>(Elem).Type, DescriptorType::CBuffer);
+ ASSERT_EQ(std::get<RootDescriptor>(Elem).Type, ResourceClass::CBuffer);
ASSERT_EQ(std::get<RootDescriptor>(Elem).Flags, DefRootDescriptorFlag);
Elem = Elements[1].getElement();
ASSERT_TRUE(std::holds_alternative<RootDescriptor>(Elem));
- ASSERT_EQ(std::get<RootDescriptor>(Elem).Type, DescriptorType::SRV);
+ ASSERT_EQ(std::get<RootDescriptor>(Elem).Type, ResourceClass::SRV);
ASSERT_EQ(std::get<RootDescriptor>(Elem).Flags, DefRootDescriptorFlag);
Elem = Elements[2].getElement();
ASSERT_TRUE(std::holds_alternative<RootDescriptor>(Elem));
- ASSERT_EQ(std::get<RootDescriptor>(Elem).Type, DescriptorType::UAV);
+ ASSERT_EQ(std::get<RootDescriptor>(Elem).Type, ResourceClass::UAV);
ASSERT_EQ(std::get<RootDescriptor>(Elem).Flags, DefRootDescriptorFlag);
auto ValidNonSamplerFlags =
@@ -714,22 +714,22 @@ TEST_F(ParseHLSLRootSignatureTest, ValidVersion10Test) {
llvm::dxbc::DescriptorRangeFlags::DataVolatile;
Elem = Elements[3].getElement();
ASSERT_TRUE(std::holds_alternative<DescriptorTableClause>(Elem));
- ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Type, ClauseType::CBuffer);
+ ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Type, ResourceClass::CBuffer);
ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Flags, ValidNonSamplerFlags);
Elem = Elements[4].getElement();
ASSERT_TRUE(std::holds_alternative<DescriptorTableClause>(Elem));
- ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Type, ClauseType::SRV);
+ ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Type, ResourceClass::SRV);
ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Flags, ValidNonSamplerFlags);
Elem = Elements[5].getElement();
ASSERT_TRUE(std::holds_alternative<DescriptorTableClause>(Elem));
- ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Type, ClauseType::UAV);
+ ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Type, ResourceClass::UAV);
ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Flags, ValidNonSamplerFlags);
Elem = Elements[6].getElement();
ASSERT_TRUE(std::holds_alternative<DescriptorTableClause>(Elem));
- ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Type, ClauseType::Sampler);
+ ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Type, ResourceClass::Sampler);
ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Flags,
llvm::dxbc::DescriptorRangeFlags::DescriptorsVolatile);
@@ -767,43 +767,43 @@ TEST_F(ParseHLSLRootSignatureTest, ValidVersion11Test) {
auto Elements = Parser.getElements();
RootElement Elem = Elements[0].getElement();
ASSERT_TRUE(std::holds_alternative<RootDescriptor>(Elem));
- ASSERT_EQ(std::get<RootDescriptor>(Elem).Type, DescriptorType::CBuffer);
+ ASSERT_EQ(std::get<RootDescriptor>(Elem).Type, ResourceClass::CBuffer);
ASSERT_EQ(std::get<RootDescriptor>(Elem).Flags,
llvm::dxbc::RootDescriptorFlags::DataStaticWhileSetAtExecute);
Elem = Elements[1].getElement();
ASSERT_TRUE(std::holds_alternative<RootDescriptor>(Elem));
- ASSERT_EQ(std::get<RootDescriptor>(Elem).Type, DescriptorType::SRV);
+ ASSERT_EQ(std::get<RootDescriptor>(Elem).Type, ResourceClass::SRV);
ASSERT_EQ(std::get<RootDescriptor>(Elem).Flags,
llvm::dxbc::RootDescriptorFlags::DataStaticWhileSetAtExecute);
Elem = Elements[2].getElement();
ASSERT_TRUE(std::holds_alternative<RootDescriptor>(Elem));
- ASSERT_EQ(std::get<RootDescriptor>(Elem).Type, DescriptorType::UAV);
+ ASSERT_EQ(std::get<RootDescriptor>(Elem).Type, ResourceClass::UAV);
ASSERT_EQ(std::get<RootDescriptor>(Elem).Flags,
llvm::dxbc::RootDescriptorFlags::DataVolatile);
Elem = Elements[3].getElement();
ASSERT_TRUE(std::holds_alternative<DescriptorTableClause>(Elem));
- ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Type, ClauseType::CBuffer);
+ ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Type, ResourceClass::CBuffer);
ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Flags,
llvm::dxbc::DescriptorRangeFlags::DataStaticWhileSetAtExecute);
Elem = Elements[4].getElement();
ASSERT_TRUE(std::holds_alternative<DescriptorTableClause>(Elem));
- ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Type, ClauseType::SRV);
+ ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Type, ResourceClass::SRV);
ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Flags,
llvm::dxbc::DescriptorRangeFlags::DataStaticWhileSetAtExecute);
Elem = Elements[5].getElement();
ASSERT_TRUE(std::holds_alternative<DescriptorTableClause>(Elem));
- ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Type, ClauseType::UAV);
+ ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Type, ResourceClass::UAV);
ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Flags,
llvm::dxbc::DescriptorRangeFlags::DataVolatile);
Elem = Elements[6].getElement();
ASSERT_TRUE(std::holds_alternative<DescriptorTableClause>(Elem));
- ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Type, ClauseType::Sampler);
+ ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Type, ResourceClass::Sampler);
ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Flags,
llvm::dxbc::DescriptorRangeFlags::None);
diff --git a/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h b/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h
index e44612af071bc..87777fddc9157 100644
--- a/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h
+++ b/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h
@@ -42,10 +42,9 @@ struct RootConstants {
dxbc::ShaderVisibility Visibility = dxbc::ShaderVisibility::All;
};
-enum class DescriptorType : uint8_t { SRV = 0, UAV, CBuffer };
// Models RootDescriptor : CBV | SRV | UAV, by collecting like parameters
struct RootDescriptor {
- DescriptorType Type;
+ dxil::ResourceClass Type;
Register Reg;
uint32_t Space = 0;
dxbc::ShaderVisibility Visibility = dxbc::ShaderVisibility::All;
@@ -60,13 +59,16 @@ struct RootDescriptor {
assert(Version == llvm::dxbc::RootSignatureVersion::V1_1 &&
"Specified an invalid root signature version");
switch (Type) {
- case DescriptorType::CBuffer:
- case DescriptorType::SRV:
+ case dxil::ResourceClass::CBuffer:
+ case dxil::ResourceClass::SRV:
Flags = dxbc::RootDescriptorFlags::DataStaticWhileSetAtExecute;
break;
- case DescriptorType::UAV:
+ case dxil::ResourceClass::UAV:
Flags = dxbc::RootDescriptorFlags::DataVolatile;
break;
+ case dxil::ResourceClass::Sampler:
+ llvm_unreachable(
+ "ResourceClass::Sampler is not valid for RootDescriptors");
}
}
};
@@ -82,9 +84,8 @@ struct DescriptorTable {
static const uint32_t NumDescriptorsUnbounded = 0xffffffff;
static const uint32_t DescriptorTableOffsetAppend = 0xffffffff;
// Models DTClause : CBV | SRV | UAV | Sampler, by collecting like parameters
-using ClauseType = llvm::dxil::ResourceClass;
struct DescriptorTableClause {
- ClauseType Type;
+ dxil::ResourceClass Type;
Register Reg;
uint32_t NumDescriptors = 1;
uint32_t Space = 0;
@@ -94,7 +95,7 @@ struct DescriptorTableClause {
void setDefaultFlags(dxbc::RootSignatureVersion Version) {
if (Version == dxbc::RootSignatureVersion::V1_0) {
Flags = dxbc::DescriptorRangeFlags::DescriptorsVolatile;
- if (Type != ClauseType::Sampler)
+ if (Type != dxil::ResourceClass::Sampler)
Flags |= dxbc::DescriptorRangeFlags::DataVolatile;
return;
}
@@ -102,14 +103,14 @@ struct DescriptorTableClause {
assert(Version == dxbc::RootSignatureVersion::V1_1 &&
"Specified an invalid root signature version");
switch (Type) {
- case ClauseType::CBuffer:
- case ClauseType::SRV:
+ case dxil::ResourceClass::CBuffer:
+ case dxil::ResourceClass::SRV:
Flags = dxbc::DescriptorRangeFlags::DataStaticWhileSetAtExecute;
break;
- case ClauseType::UAV:
+ case dxil::ResourceClass::UAV:
Flags = dxbc::DescriptorRangeFlags::DataVolatile;
break;
- case ClauseType::Sampler:
+ case dxil::ResourceClass::Sampler:
Flags = dxbc::DescriptorRangeFlags::None;
break;
}
diff --git a/llvm/lib/Frontend/HLSL/HLSLRootSignature.cpp b/llvm/lib/Frontend/HLSL/HLSLRootSignature.cpp
index 050cc46e8c9b0..14c729b1ad97b 100644
--- a/llvm/lib/Frontend/HLSL/HLSLRootSignature.cpp
+++ b/llvm/lib/Frontend/HLSL/HLSLRootSignature.cpp
@@ -92,9 +92,9 @@ static raw_ostream &operator<<(raw_ostream &OS,
return OS;
}
-static raw_ostream &operator<<(raw_ostream &OS, const ClauseType &Type) {
- OS << enumToStringRef(dxil::ResourceClass(llvm::to_underlying(Type)),
- dxil::getResourceClasses());
+static raw_ostream &operator<<(raw_ostream &OS,
+ const dxil::ResourceClass &Type) {
+ OS << enumToStringRef(Type, dxil::getResourceClasses());
return OS;
}
@@ -153,8 +153,7 @@ raw_ostream &operator<<(raw_ostream &OS, const DescriptorTableClause &Clause) {
}
raw_ostream &operator<<(raw_ostream &OS, const RootDescriptor &Descriptor) {
- ClauseType Type = ClauseType(llvm::to_underlying(Descriptor.Type));
- OS << "Root" << Type << "(" << Descriptor.Reg
+ OS << "Root" << Descriptor.Type << "(" << Descriptor.Reg
<< ", space = " << Descriptor.Space
<< ", visibility = " << Descriptor.Visibility
<< ", flags = " << Descriptor.Flags << ")";
diff --git a/llvm/lib/Frontend/HLSL/RootSignatureMetadata.cpp b/llvm/lib/Frontend/HLSL/RootSignatureMetadata.cpp
index 157bfc665b207..ae58d80e12d5f 100644
--- a/llvm/lib/Frontend/HLSL/RootSignatureMetadata.cpp
+++ b/llvm/lib/Frontend/HLSL/RootSignatureMetadata.cpp
@@ -120,8 +120,7 @@ MDNode *MetadataBuilder::BuildRootConstants(const RootConstants &Constants) {
MDNode *MetadataBuilder::BuildRootDescriptor(const RootDescriptor &Descriptor) {
IRBuilder<> Builder(Ctx);
StringRef ResName =
- enumToStringRef(dxil::ResourceClass(to_underlying(Descriptor.Type)),
- dxil::getResourceClasses());
+ enumToStringRef(Descriptor.Type, dxil::getResourceClasses());
assert(!ResName.empty() && "Provided an invalid Resource Class");
SmallString<7> Name({"Root", ResName});
Metadata *Operands[] = {
@@ -161,9 +160,7 @@ MDNode *MetadataBuilder::BuildDescriptorTable(const DescriptorTable &Table) {
MDNode *MetadataBuilder::BuildDescriptorTableClause(
const DescriptorTableClause &Clause) {
IRBuilder<> Builder(Ctx);
- StringRef ResName =
- enumToStringRef(dxil::ResourceClass(to_underlying(Clause.Type)),
- dxil::getResourceClasses());
+ StringRef ResName = enumToStringRef(Clause.Type, dxil::getResourceClasses());
assert(!ResName.empty() && "Provided an invalid Resource Class");
Metadata *Operands[] = {
MDString::get(Ctx, ResName),
|
@llvm/pr-subscribers-hlsl Author: Damyan Pepper (damyanp) ChangesRemove:
Although these were introduced to make the HLSL Closes #153890 Full diff: https://github.com/llvm/llvm-project/pull/153909.diff 5 Files Affected:
diff --git a/clang/lib/Parse/ParseHLSLRootSignature.cpp b/clang/lib/Parse/ParseHLSLRootSignature.cpp
index 98dc458f7adc5..5490c61f52356 100644
--- a/clang/lib/Parse/ParseHLSLRootSignature.cpp
+++ b/clang/lib/Parse/ParseHLSLRootSignature.cpp
@@ -234,15 +234,15 @@ std::optional<RootDescriptor> RootSignatureParser::parseRootDescriptor() {
default:
llvm_unreachable("Switch for consumed token was not provided");
case TokenKind::kw_CBV:
- Descriptor.Type = DescriptorType::CBuffer;
+ Descriptor.Type = ResourceClass::CBuffer;
ExpectedReg = TokenKind::bReg;
break;
case TokenKind::kw_SRV:
- Descriptor.Type = DescriptorType::SRV;
+ Descriptor.Type = ResourceClass::SRV;
ExpectedReg = TokenKind::tReg;
break;
case TokenKind::kw_UAV:
- Descriptor.Type = DescriptorType::UAV;
+ Descriptor.Type = ResourceClass::UAV;
ExpectedReg = TokenKind::uReg;
break;
}
@@ -360,19 +360,19 @@ RootSignatureParser::parseDescriptorTableClause() {
default:
llvm_unreachable("Switch for consumed token was not provided");
case TokenKind::kw_CBV:
- Clause.Type = ClauseType::CBuffer;
+ Clause.Type = ResourceClass::CBuffer;
ExpectedReg = TokenKind::bReg;
break;
case TokenKind::kw_SRV:
- Clause.Type = ClauseType::SRV;
+ Clause.Type = ResourceClass::SRV;
ExpectedReg = TokenKind::tReg;
break;
case TokenKind::kw_UAV:
- Clause.Type = ClauseType::UAV;
+ Clause.Type = ResourceClass::UAV;
ExpectedReg = TokenKind::uReg;
break;
case TokenKind::kw_Sampler:
- Clause.Type = ClauseType::Sampler;
+ Clause.Type = ResourceClass::Sampler;
ExpectedReg = TokenKind::sReg;
break;
}
diff --git a/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp b/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
index 44f6b0469f38e..44c0978a243bc 100644
--- a/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
+++ b/clang/unittests/Parse/ParseHLSLRootSignatureTest.cpp
@@ -180,7 +180,7 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseDTClausesTest) {
// First Descriptor Table with 4 elements
RootElement Elem = Elements[0].getElement();
ASSERT_TRUE(std::holds_alternative<DescriptorTableClause>(Elem));
- ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Type, ClauseType::CBuffer);
+ ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Type, ResourceClass::CBuffer);
ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Reg.ViewType,
RegisterType::BReg);
ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Reg.Number, 0u);
@@ -193,7 +193,7 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseDTClausesTest) {
Elem = Elements[1].getElement();
ASSERT_TRUE(std::holds_alternative<DescriptorTableClause>(Elem));
- ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Type, ClauseType::SRV);
+ ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Type, ResourceClass::SRV);
ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Reg.ViewType,
RegisterType::TReg);
ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Reg.Number, 42u);
@@ -205,7 +205,7 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseDTClausesTest) {
Elem = Elements[2].getElement();
ASSERT_TRUE(std::holds_alternative<DescriptorTableClause>(Elem));
- ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Type, ClauseType::Sampler);
+ ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Type, ResourceClass::Sampler);
ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Reg.ViewType,
RegisterType::SReg);
ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Reg.Number, 987u);
@@ -218,7 +218,7 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseDTClausesTest) {
Elem = Elements[3].getElement();
ASSERT_TRUE(std::holds_alternative<DescriptorTableClause>(Elem));
- ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Type, ClauseType::UAV);
+ ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Type, ResourceClass::UAV);
ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Reg.ViewType,
RegisterType::UReg);
ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Reg.Number, 4294967294u);
@@ -445,7 +445,7 @@ TEST_F(ParseHLSLRootSignatureTest, ValidSamplerFlagsTest) {
auto Elements = Parser.getElements();
RootElement Elem = Elements[0].getElement();
ASSERT_TRUE(std::holds_alternative<DescriptorTableClause>(Elem));
- ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Type, ClauseType::Sampler);
+ ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Type, ResourceClass::Sampler);
auto ValidSamplerFlags =
llvm::dxbc::DescriptorRangeFlags::DescriptorsVolatile;
ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Flags, ValidSamplerFlags);
@@ -591,7 +591,7 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseRootDescriptorsTest) {
RootElement Elem = Elements[0].getElement();
ASSERT_TRUE(std::holds_alternative<RootDescriptor>(Elem));
- ASSERT_EQ(std::get<RootDescriptor>(Elem).Type, DescriptorType::CBuffer);
+ ASSERT_EQ(std::get<RootDescriptor>(Elem).Type, ResourceClass::CBuffer);
ASSERT_EQ(std::get<RootDescriptor>(Elem).Reg.ViewType, RegisterType::BReg);
ASSERT_EQ(std::get<RootDescriptor>(Elem).Reg.Number, 0u);
ASSERT_EQ(std::get<RootDescriptor>(Elem).Space, 0u);
@@ -602,7 +602,7 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseRootDescriptorsTest) {
Elem = Elements[1].getElement();
ASSERT_TRUE(std::holds_alternative<RootDescriptor>(Elem));
- ASSERT_EQ(std::get<RootDescriptor>(Elem).Type, DescriptorType::SRV);
+ ASSERT_EQ(std::get<RootDescriptor>(Elem).Type, ResourceClass::SRV);
ASSERT_EQ(std::get<RootDescriptor>(Elem).Reg.ViewType, RegisterType::TReg);
ASSERT_EQ(std::get<RootDescriptor>(Elem).Reg.Number, 42u);
ASSERT_EQ(std::get<RootDescriptor>(Elem).Space, 4u);
@@ -616,7 +616,7 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseRootDescriptorsTest) {
Elem = Elements[2].getElement();
ASSERT_TRUE(std::holds_alternative<RootDescriptor>(Elem));
- ASSERT_EQ(std::get<RootDescriptor>(Elem).Type, DescriptorType::UAV);
+ ASSERT_EQ(std::get<RootDescriptor>(Elem).Type, ResourceClass::UAV);
ASSERT_EQ(std::get<RootDescriptor>(Elem).Reg.ViewType, RegisterType::UReg);
ASSERT_EQ(std::get<RootDescriptor>(Elem).Reg.Number, 34893247u);
ASSERT_EQ(std::get<RootDescriptor>(Elem).Space, 0u);
@@ -628,7 +628,7 @@ TEST_F(ParseHLSLRootSignatureTest, ValidParseRootDescriptorsTest) {
RootDescriptorFlags::DataVolatile);
Elem = Elements[3].getElement();
- ASSERT_EQ(std::get<RootDescriptor>(Elem).Type, DescriptorType::CBuffer);
+ ASSERT_EQ(std::get<RootDescriptor>(Elem).Type, ResourceClass::CBuffer);
ASSERT_EQ(std::get<RootDescriptor>(Elem).Reg.ViewType, RegisterType::BReg);
ASSERT_EQ(std::get<RootDescriptor>(Elem).Reg.Number, 0u);
ASSERT_EQ(std::get<RootDescriptor>(Elem).Space, 0u);
@@ -696,17 +696,17 @@ TEST_F(ParseHLSLRootSignatureTest, ValidVersion10Test) {
auto DefRootDescriptorFlag = llvm::dxbc::RootDescriptorFlags::DataVolatile;
RootElement Elem = Elements[0].getElement();
ASSERT_TRUE(std::holds_alternative<RootDescriptor>(Elem));
- ASSERT_EQ(std::get<RootDescriptor>(Elem).Type, DescriptorType::CBuffer);
+ ASSERT_EQ(std::get<RootDescriptor>(Elem).Type, ResourceClass::CBuffer);
ASSERT_EQ(std::get<RootDescriptor>(Elem).Flags, DefRootDescriptorFlag);
Elem = Elements[1].getElement();
ASSERT_TRUE(std::holds_alternative<RootDescriptor>(Elem));
- ASSERT_EQ(std::get<RootDescriptor>(Elem).Type, DescriptorType::SRV);
+ ASSERT_EQ(std::get<RootDescriptor>(Elem).Type, ResourceClass::SRV);
ASSERT_EQ(std::get<RootDescriptor>(Elem).Flags, DefRootDescriptorFlag);
Elem = Elements[2].getElement();
ASSERT_TRUE(std::holds_alternative<RootDescriptor>(Elem));
- ASSERT_EQ(std::get<RootDescriptor>(Elem).Type, DescriptorType::UAV);
+ ASSERT_EQ(std::get<RootDescriptor>(Elem).Type, ResourceClass::UAV);
ASSERT_EQ(std::get<RootDescriptor>(Elem).Flags, DefRootDescriptorFlag);
auto ValidNonSamplerFlags =
@@ -714,22 +714,22 @@ TEST_F(ParseHLSLRootSignatureTest, ValidVersion10Test) {
llvm::dxbc::DescriptorRangeFlags::DataVolatile;
Elem = Elements[3].getElement();
ASSERT_TRUE(std::holds_alternative<DescriptorTableClause>(Elem));
- ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Type, ClauseType::CBuffer);
+ ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Type, ResourceClass::CBuffer);
ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Flags, ValidNonSamplerFlags);
Elem = Elements[4].getElement();
ASSERT_TRUE(std::holds_alternative<DescriptorTableClause>(Elem));
- ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Type, ClauseType::SRV);
+ ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Type, ResourceClass::SRV);
ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Flags, ValidNonSamplerFlags);
Elem = Elements[5].getElement();
ASSERT_TRUE(std::holds_alternative<DescriptorTableClause>(Elem));
- ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Type, ClauseType::UAV);
+ ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Type, ResourceClass::UAV);
ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Flags, ValidNonSamplerFlags);
Elem = Elements[6].getElement();
ASSERT_TRUE(std::holds_alternative<DescriptorTableClause>(Elem));
- ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Type, ClauseType::Sampler);
+ ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Type, ResourceClass::Sampler);
ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Flags,
llvm::dxbc::DescriptorRangeFlags::DescriptorsVolatile);
@@ -767,43 +767,43 @@ TEST_F(ParseHLSLRootSignatureTest, ValidVersion11Test) {
auto Elements = Parser.getElements();
RootElement Elem = Elements[0].getElement();
ASSERT_TRUE(std::holds_alternative<RootDescriptor>(Elem));
- ASSERT_EQ(std::get<RootDescriptor>(Elem).Type, DescriptorType::CBuffer);
+ ASSERT_EQ(std::get<RootDescriptor>(Elem).Type, ResourceClass::CBuffer);
ASSERT_EQ(std::get<RootDescriptor>(Elem).Flags,
llvm::dxbc::RootDescriptorFlags::DataStaticWhileSetAtExecute);
Elem = Elements[1].getElement();
ASSERT_TRUE(std::holds_alternative<RootDescriptor>(Elem));
- ASSERT_EQ(std::get<RootDescriptor>(Elem).Type, DescriptorType::SRV);
+ ASSERT_EQ(std::get<RootDescriptor>(Elem).Type, ResourceClass::SRV);
ASSERT_EQ(std::get<RootDescriptor>(Elem).Flags,
llvm::dxbc::RootDescriptorFlags::DataStaticWhileSetAtExecute);
Elem = Elements[2].getElement();
ASSERT_TRUE(std::holds_alternative<RootDescriptor>(Elem));
- ASSERT_EQ(std::get<RootDescriptor>(Elem).Type, DescriptorType::UAV);
+ ASSERT_EQ(std::get<RootDescriptor>(Elem).Type, ResourceClass::UAV);
ASSERT_EQ(std::get<RootDescriptor>(Elem).Flags,
llvm::dxbc::RootDescriptorFlags::DataVolatile);
Elem = Elements[3].getElement();
ASSERT_TRUE(std::holds_alternative<DescriptorTableClause>(Elem));
- ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Type, ClauseType::CBuffer);
+ ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Type, ResourceClass::CBuffer);
ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Flags,
llvm::dxbc::DescriptorRangeFlags::DataStaticWhileSetAtExecute);
Elem = Elements[4].getElement();
ASSERT_TRUE(std::holds_alternative<DescriptorTableClause>(Elem));
- ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Type, ClauseType::SRV);
+ ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Type, ResourceClass::SRV);
ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Flags,
llvm::dxbc::DescriptorRangeFlags::DataStaticWhileSetAtExecute);
Elem = Elements[5].getElement();
ASSERT_TRUE(std::holds_alternative<DescriptorTableClause>(Elem));
- ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Type, ClauseType::UAV);
+ ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Type, ResourceClass::UAV);
ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Flags,
llvm::dxbc::DescriptorRangeFlags::DataVolatile);
Elem = Elements[6].getElement();
ASSERT_TRUE(std::holds_alternative<DescriptorTableClause>(Elem));
- ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Type, ClauseType::Sampler);
+ ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Type, ResourceClass::Sampler);
ASSERT_EQ(std::get<DescriptorTableClause>(Elem).Flags,
llvm::dxbc::DescriptorRangeFlags::None);
diff --git a/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h b/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h
index e44612af071bc..87777fddc9157 100644
--- a/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h
+++ b/llvm/include/llvm/Frontend/HLSL/HLSLRootSignature.h
@@ -42,10 +42,9 @@ struct RootConstants {
dxbc::ShaderVisibility Visibility = dxbc::ShaderVisibility::All;
};
-enum class DescriptorType : uint8_t { SRV = 0, UAV, CBuffer };
// Models RootDescriptor : CBV | SRV | UAV, by collecting like parameters
struct RootDescriptor {
- DescriptorType Type;
+ dxil::ResourceClass Type;
Register Reg;
uint32_t Space = 0;
dxbc::ShaderVisibility Visibility = dxbc::ShaderVisibility::All;
@@ -60,13 +59,16 @@ struct RootDescriptor {
assert(Version == llvm::dxbc::RootSignatureVersion::V1_1 &&
"Specified an invalid root signature version");
switch (Type) {
- case DescriptorType::CBuffer:
- case DescriptorType::SRV:
+ case dxil::ResourceClass::CBuffer:
+ case dxil::ResourceClass::SRV:
Flags = dxbc::RootDescriptorFlags::DataStaticWhileSetAtExecute;
break;
- case DescriptorType::UAV:
+ case dxil::ResourceClass::UAV:
Flags = dxbc::RootDescriptorFlags::DataVolatile;
break;
+ case dxil::ResourceClass::Sampler:
+ llvm_unreachable(
+ "ResourceClass::Sampler is not valid for RootDescriptors");
}
}
};
@@ -82,9 +84,8 @@ struct DescriptorTable {
static const uint32_t NumDescriptorsUnbounded = 0xffffffff;
static const uint32_t DescriptorTableOffsetAppend = 0xffffffff;
// Models DTClause : CBV | SRV | UAV | Sampler, by collecting like parameters
-using ClauseType = llvm::dxil::ResourceClass;
struct DescriptorTableClause {
- ClauseType Type;
+ dxil::ResourceClass Type;
Register Reg;
uint32_t NumDescriptors = 1;
uint32_t Space = 0;
@@ -94,7 +95,7 @@ struct DescriptorTableClause {
void setDefaultFlags(dxbc::RootSignatureVersion Version) {
if (Version == dxbc::RootSignatureVersion::V1_0) {
Flags = dxbc::DescriptorRangeFlags::DescriptorsVolatile;
- if (Type != ClauseType::Sampler)
+ if (Type != dxil::ResourceClass::Sampler)
Flags |= dxbc::DescriptorRangeFlags::DataVolatile;
return;
}
@@ -102,14 +103,14 @@ struct DescriptorTableClause {
assert(Version == dxbc::RootSignatureVersion::V1_1 &&
"Specified an invalid root signature version");
switch (Type) {
- case ClauseType::CBuffer:
- case ClauseType::SRV:
+ case dxil::ResourceClass::CBuffer:
+ case dxil::ResourceClass::SRV:
Flags = dxbc::DescriptorRangeFlags::DataStaticWhileSetAtExecute;
break;
- case ClauseType::UAV:
+ case dxil::ResourceClass::UAV:
Flags = dxbc::DescriptorRangeFlags::DataVolatile;
break;
- case ClauseType::Sampler:
+ case dxil::ResourceClass::Sampler:
Flags = dxbc::DescriptorRangeFlags::None;
break;
}
diff --git a/llvm/lib/Frontend/HLSL/HLSLRootSignature.cpp b/llvm/lib/Frontend/HLSL/HLSLRootSignature.cpp
index 050cc46e8c9b0..14c729b1ad97b 100644
--- a/llvm/lib/Frontend/HLSL/HLSLRootSignature.cpp
+++ b/llvm/lib/Frontend/HLSL/HLSLRootSignature.cpp
@@ -92,9 +92,9 @@ static raw_ostream &operator<<(raw_ostream &OS,
return OS;
}
-static raw_ostream &operator<<(raw_ostream &OS, const ClauseType &Type) {
- OS << enumToStringRef(dxil::ResourceClass(llvm::to_underlying(Type)),
- dxil::getResourceClasses());
+static raw_ostream &operator<<(raw_ostream &OS,
+ const dxil::ResourceClass &Type) {
+ OS << enumToStringRef(Type, dxil::getResourceClasses());
return OS;
}
@@ -153,8 +153,7 @@ raw_ostream &operator<<(raw_ostream &OS, const DescriptorTableClause &Clause) {
}
raw_ostream &operator<<(raw_ostream &OS, const RootDescriptor &Descriptor) {
- ClauseType Type = ClauseType(llvm::to_underlying(Descriptor.Type));
- OS << "Root" << Type << "(" << Descriptor.Reg
+ OS << "Root" << Descriptor.Type << "(" << Descriptor.Reg
<< ", space = " << Descriptor.Space
<< ", visibility = " << Descriptor.Visibility
<< ", flags = " << Descriptor.Flags << ")";
diff --git a/llvm/lib/Frontend/HLSL/RootSignatureMetadata.cpp b/llvm/lib/Frontend/HLSL/RootSignatureMetadata.cpp
index 157bfc665b207..ae58d80e12d5f 100644
--- a/llvm/lib/Frontend/HLSL/RootSignatureMetadata.cpp
+++ b/llvm/lib/Frontend/HLSL/RootSignatureMetadata.cpp
@@ -120,8 +120,7 @@ MDNode *MetadataBuilder::BuildRootConstants(const RootConstants &Constants) {
MDNode *MetadataBuilder::BuildRootDescriptor(const RootDescriptor &Descriptor) {
IRBuilder<> Builder(Ctx);
StringRef ResName =
- enumToStringRef(dxil::ResourceClass(to_underlying(Descriptor.Type)),
- dxil::getResourceClasses());
+ enumToStringRef(Descriptor.Type, dxil::getResourceClasses());
assert(!ResName.empty() && "Provided an invalid Resource Class");
SmallString<7> Name({"Root", ResName});
Metadata *Operands[] = {
@@ -161,9 +160,7 @@ MDNode *MetadataBuilder::BuildDescriptorTable(const DescriptorTable &Table) {
MDNode *MetadataBuilder::BuildDescriptorTableClause(
const DescriptorTableClause &Clause) {
IRBuilder<> Builder(Ctx);
- StringRef ResName =
- enumToStringRef(dxil::ResourceClass(to_underlying(Clause.Type)),
- dxil::getResourceClasses());
+ StringRef ResName = enumToStringRef(Clause.Type, dxil::getResourceClasses());
assert(!ResName.empty() && "Provided an invalid Resource Class");
Metadata *Operands[] = {
MDString::get(Ctx, ResName),
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, it is a codebase I am not familiar with though
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. As noted, they were introduced to make an enforced difference between the two types. But given it has created more confusion, I'm happy to merge them together.
Remove:
Although these were introduced to make the HLSL root signature handling code a bit cleaner, they were ultimately causing confusion as they appeared to be unique enums that needed to be converted between each other.
Closes #153890